Update get cmd for 'all' when retrieving all data-types. #541
Conversation
Thank you for the pull request!The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Data rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist |
| @@ -342,7 +342,7 @@ def query_data( | |||
| print( | |||
| f"Successfully queried and formatted data for {lang.title()} {target_type}." | |||
| ) | |||
| return {"success": True, "skipped": False} | |||
| # return {"success": True, "skipped": False} | |||
There was a problem hiding this comment.
Because we are using return inside that loop, the function will exit on the first iteration that reaches any return statement. As a result, tqdm never finishes iterating through its items.
Also in tqdm never ends as 100%. like this-
There was a problem hiding this comment.
Had to check #537 for iterating through all the language.
| @@ -208,35 +208,35 @@ def prompt_user_download_all(): | |||
| f"Updating data for language(s): {language.title()}; data type(s): {data_type.capitalize()}" | |||
| ) | |||
| existing_files = list(Path(output_dir).glob(f"{language}/{data_type}.json")) | |||
| if existing_files: | |||
| if existing_files and not overwrite: | |||
There was a problem hiding this comment.
If user gives -o for overwrite enable, it still asks user input. Like this in first cmd-
The second cmd is the outputted result after fixing.
| language_qid = language | ||
| if ( | ||
| language is not None | ||
| and (language.startswith("Q") or language.startswith("q")) |
There was a problem hiding this comment.
Added this in as lower casing the language was actually breaking the ability to pass a QID as a language :)
| @@ -547,6 +544,9 @@ def format_sublanguage_name(lang, language_metadata=_languages): | |||
| > format_sublanguage_name("english", language_metadata) | |||
| 'English' | |||
| """ | |||
| if (lang.startswith("Q") or lang.startswith("q")) and lang[1:].isdigit(): | |||
There was a problem hiding this comment.
The next part of fixing passing QIDs for languages. There may need to be more fixes related to this, @axif0! Goal was getting total to work :)
There was a problem hiding this comment.
Thanks for this, @axif0! Let me know if you're free for the hour before the sync this Saturday :) Would be great if we could meet and try out all of the functionality and document what's working or needs a bit of polishing 😊
Amazing progress so far! Really thanks so much :)
|
Looking forward to our meeting an hour before the sync this Saturday. |
|
Me too, @axif0! 😊 |
Contributor checklist
pytestcommand as directed in the testing section of the contributing guideDescription
Added get cmd for 'all' when retrieving all data-types.
Related issue
get --data-type (-dt) -afunctionality #522